-
Couldn't load subscription status.
- Fork 0
NAS and Job Feign Client #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| * @author Administrator | ||
| * | ||
| */ | ||
| @Lazy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Job is needed while performing most of ontap operations, so, is Lazy needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this is in use, it will get initialise and next time it will be available so it will not go to initilize phase again so we can use lazy annotation, only SVM and cluster i did not require lazy bcz we need disaggregated and protocol info
| //File Operations | ||
|
|
||
| @RequestMapping(method = RequestMethod.GET, value="/{volume.uuid}/files/{path}") | ||
| OntapResponse<FileInfo> getFileResponse(URI uri, @RequestHeader("Authorization") String header, @PathVariable(name = "volume.uuid", required = true) String volumeUUID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember seeing OnTapResponse, was this changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes changed to OntapResponse
| import org.springframework.web.bind.annotation.RequestMethod; | ||
|
|
||
|
|
||
| @FeignClient(name = "VolumeClient", url = "https://{clusterIP}/api/storage/volumes", configuration = FeignConfiguration.class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess here we can use Lazy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
| private Integer index = null; | ||
|
|
||
| public enum ProtocolsEnum { | ||
| any("any"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we leave them as is? Or shall we have only the ones that we would be supporting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could remove this too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
| return o.toString().replace("\n", "\n "); | ||
| } | ||
|
|
||
| public static class ExportClient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets put this somewhere in beginning itself, instead at the EOF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shifted little up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Volume.java doesn't have a reference to Aggregate.java, please include it, if we are removing this
Also, can you please see if 'size' could be added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some minor comments, approving them with this since they are not blocker. pls ensure to take up them in next PR
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash( id, name, rules, svm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hashcode and equals should use same set of attributes to evaluate the usecase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, will include in next PR
| public enum ProtocolsEnum { | ||
| any("any"), | ||
|
|
||
| nfs("nfs"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nfs is nfs3 only, do we really need nfs3 separately ?
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
…ommits. # This is the 1st commit message: CSTACKEX-25: Basic class structure # This is the commit message #2: Add PrimaryStoragePool base code # This is the commit message #3: CSTACKEX-25: Create Volume code basic code added # This is the commit message #4: CSTACKEX-25: additional logic for Primary storage pool creation # This is the commit message #5: CSTACKEX-29 Cluster, SVM and Aggr Feign Client # This is the commit message #6: CSTACKEX-29 Added License Info # This is the commit message #7: CSTACKEX-29 Resolve Review Comments # This is the commit message #8: CSTACKEX-29 Resolve Style check issues � This is the commit message #9: CSTACKEX-29 Resolve Style check issues � This is the commit message #10: CSTACKEX-29 Resolve Precommits Issues # This is the commit message #11: CSTACKEX-29 Resolve Precommits Issues
Description
NAS and Job Feign Client with POJOs
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?